Validate DatabricksSQLStatementsSensor exclusivity at __init__ - #70831

The split is right and the follow-up commit lands everything that was asked for — ValueError, the updated pytest.raises, and the ratchet drop to ::3. Two observations inline and one process note below; none are blockers.

I checked a few things rather than assuming them, and they hold up:

• #70340 (which moved the check into execute()) landed after the last provider release prep, so it is unreleased. Against released 7.18.0 this PR is a net no-op on placement — the genuinely new behaviour is the polarity and the exception type, which matches how the description frames it.
• Pre-#70340 the __init__ check was if statement and statement_id raising AirflowException. So this really isn't a straight revert, as stated.
• DatabricksSQLStatementsOperator takes statement: str as required and does not list it in template_fields, so there is no parallel site needing the same treatment. Nothing sibling is left half-fixed.

Process — the PR title ships into the changelog verbatim

The title currently ends in a literal ellipsis:

Rewrite DatabricksSQLStatementsSensor exclusivity check and move back…

Databricks changelogs are regenerated from git log by the release manager rather than from newsfragments (see providers/AGENTS.md), so the truncation would land as-is in providers/databricks/docs/changelog.rst. Worth retitling to something that stands on its own and stays under 70 characters — for example:

Validate DatabricksSQLStatementsSensor exclusivity at __init__

Please retitle the commit as well as the PR, since the commit message is what the changelog is generated from.

On your question about the remaining AirflowExceptions

Could the AirflowException for both checks mentioned in this PR be converted to ValueError raises as part of this PR or should they remain as is?

Both readings are defensible, and the ratchet permits either. Neither warehouse_id must be provided. nor One of either statement or statement_id must be provided. is modified by this diff, so leaving them is not a violation — the check-no-new-airflow-exceptions hook only guards new usages.

That said, I'd convert them here. The same "you're already touching this function" argument that applied to the relocated line applies to its two neighbours, and the current state leaves an odd surface for anyone writing an except block: passing both statement and statement_id raises ValueError, while passing neither raises AirflowException, for what a Dag author experiences as the same class of mistake. Both are argument-provision errors and ValueError fits both. That would take the ratchet entry from ::3 to ::1.

If you'd rather keep this PR tightly scoped to what was asked for, a follow-up is fine too — just say which way you're going so the ratchet line doesn't get churned twice.

This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The findings
below are observations, not blockers; an Apache Airflow
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Apache Airflow handles maintainer review:
contributing-docs/05_pull_requests.rst.

Drafted-by: Claude Code (Opus 5); reviewed by @shahar1 before posting

Thanks for the clear write-up — the reasoning matches the criterion in #70503 exactly, and it follows the same restoration already merged for the Repos operators in #70551.

I ran a differential against a live SQL warehouse. Provider 7.18.1 already contains #70340, so the unpatched wheel reproduces current main and the patched one carries your two edits (Airflow 3.2.2, real warehouse_id, real statements executed):

The first row is an argument for this change that the description does not make: on main, supplying both arguments produces no error at all once one of them renders to None, and the sensor quietly runs the other. Catching that at parse time is a real improvement rather than a relocation — worth saying in the description.

The second row is a user-visible behaviour change: statement_id="" alongside a statement works today (confirmed end to end) and becomes a Dag import error here. Your parametrization shows it is deliberate, so this is only about making it visible to whoever reads the changelog later.

Nits inline, none blocking.

Drafted-by: Claude Code (Opus 5); reviewed by @moomindani before posting

This is the stronger form of the test, not the dead-code one — thanks for reworking it. Approving on 95ab90dd.

I checked it discriminates, which was the point of the last exchange: moving the exclusivity check from __init__ into execute() while keeping is not None fails both *_validated_at_init tests, and deleting the render_template_fields call from the execute-side test fails that one. The dead-code variant would have passed the first mutation.

The only source change since the commit I reviewed on 2026-08-19 is the message string, so the live-warehouse differential I reported then still describes this head.

Drafted-by: Claude Code (Opus 5); reviewed by @moomindani before posting

The ValueError narrowing and ratchet drop I asked for on 2026-08-01 are in, and the test now discriminates in the direction that matters. Locally: 30/30 sensor tests pass, validate_operators_init.py exits 0 on the file, ruff clean.

One substantive thing to add, and it is a fact that moved while this PR was open rather than anything you got wrong.

The 2026-08-01 review reasoned that #70340 was unreleased, so this PR was "a net no-op on placement" against released 7.18.0. That held at the time. But providers-databricks/7.19.0 was tagged on 2026-08-19 and its tree contains #70340 — the released sensor checks exclusivity in execute() with truthiness. So the differential measured against a live warehouse is a change against a released version:

• statement="SELECT 1" with statement_id="" runs the statement in 7.19.0, and becomes a Dag import error here.
• Same for a statement_id template that renders to None under render_template_as_native_obj=True.
• The three AirflowExceptions become ValueError, so an except AirflowException around these stops matching.

The tightening itself is right and #70503 sanctions it explicitly ("strictly stricter — it also rejects field=\"\" alongside a second argument"). This is only about the note. Providers take no newsfragment, and providers/databricks/docs/changelog.rst asks for exactly this — a note just below the Changelog header for a breaking change that needs an explanation. Same shape as the ComprehendCreateDocumentClassifierOperator warning at the top of the amazon changelog.

Insert after line 28 (the blank line under ---------), above 7.19.0:

.. warning::
  ``DatabricksSQLStatementsSensor`` now rejects the combination of ``statement`` and
  ``statement_id`` when the Dag is parsed rather than when the task runs, and the check is
  ``is not None``, so an empty string counts as provided. ``statement="SELECT 1"`` together
  with ``statement_id=""`` — or with a ``statement_id`` template that renders to ``None``
  under ``render_template_as_native_obj=True`` — ran the statement in 7.19.0 and now raises
  at Dag parse time. Pass exactly one of the two and omit the other entirely instead of
  passing an empty value. The argument-validation errors in this sensor are also now
  ``ValueError`` rather than ``AirflowException``.

Two asks, then, and I'll merge once both are in:

1. The changelog warning above.
2. A rebase. The branch is 128 commits behind main and the green CI run is against a base from before the 7.19.0 release — worth a fresh run on current main before this goes in.

git fetch upstream main
git rebase upstream/main
git push --force-with-lease origin providers/rewrite_databrickssqlstatementssensor_exclusivity_check

Nothing else from me — the __init__/execute() split, the comment explaining it, and the constructor-only assertion are all settled and I do not want to re-open them.

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting